Merge Reader Dashboard into Profile Portfolio tab#690
Conversation
- Portfolio tab now includes paginated Trading History with buy/sell badges, storyline titles, USD values — visible for all profiles (public on-chain data) - Donation history (given as reader) gated to own profile only - /reader now redirects to /profile/[address]?tab=portfolio Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
project7-interns
left a comment
There was a problem hiding this comment.
LGTM. Clean reader dashboard consolidation: Trading History moved to PortfolioTab (public for all profiles), Donations Given properly gated behind isOwnProfile, /reader redirects to profile. Net -162 lines. All #684 acceptance criteria met.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The redirect and public trading-history move are fine, but the merged Portfolio tab still does not carry over the full reader-dashboard behavior from #684. Two required pieces are still missing on the current head.
Findings
-
[medium] The Portfolio section still uses the older profile holdings view rather than the reader-dashboard portfolio summary, so it does not show the required 24h price-change/best-pick data from
ReaderPortfolio.- File:
src/app/profile/[address]/page.tsx:1311 - Suggestion: Port the
ReaderPortfoliobehavior intoPortfolioTab(or makeReaderPortfolioaccept a profile address and render it here) so the Profile Portfolio tab includes the same 24h change and best-pick information as the old/readerdashboard.
- File:
-
[medium] Donation history is still the old non-paginated profile list capped at 20 rows, rather than the paginated reader-dashboard donation history required by #684.
- File:
src/app/profile/[address]/page.tsx:1242 - Suggestion: Replace the current
donationsGivensummary/list with the paginated donation-history flow from the old/readerdashboard so own-profile portfolio matches the original reader dashboard behavior.
- File:
Decision
Requesting changes because the current Portfolio tab still misses required reader-dashboard functionality from #684.
… donations - Portfolio holdings now include 24h price change per token and best pick summary - Uses get24hPriceChange + getTokenTVL for reserve decimals (matches ReaderPortfolio) - Donations given now uses paginated useInfiniteQuery instead of capped query Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Re-review: both fixes confirmed. Holdings now include 24h price change per token + best pick summary (matching ReaderPortfolio). Donations given now paginated via useInfiniteQuery. LGTM.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The latest head fixes the portfolio-summary and donation-pagination gaps from the previous review, but one Portfolio-tab regression still remains on the current revision.
Findings
- [medium]
PortfolioTabstill returns the empty-state card before renderingPortfolioTradingHistory, so profiles with public trades but no current holdings or donations will incorrectly show "No holdings or donations yet" and hide the required trading-history section.- File:
src/app/profile/[address]/page.tsx:1319 - Suggestion: Include trading history in the empty-state decision, or render
PortfolioTradingHistoryoutside that early return so profiles with trades but no current holdings/donations still satisfy #684.
- File:
Decision
Requesting changes because the Portfolio tab can still drop required public trading-history data for valid profiles.
Profiles with trades but no holdings/donations were hitting the empty state early return, hiding the PortfolioTradingHistory section. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Re-review: early return removed so PortfolioTradingHistory renders for profiles with trades but no holdings/donations. Edge case fixed. LGTM.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The latest head resolves the remaining Portfolio-tab blocker from the previous review. PortfolioTradingHistory is no longer hidden behind the old empty-state early return, and the merged reader-dashboard behavior now matches the #684 requirements.
Findings
- None.
Decision
Approving because the current revision now carries over the required Portfolio summary, paginated donation history, public trading history, and /reader redirect behavior into the profile page.
Summary
/readernow redirects to/profile/[address]?tab=portfolioFixes #684
Test plan
/readerredirects to profilenpm run buildpasses ✅🤖 Generated with Claude Code